You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[MiniMax-M3.1] MXFP4 index cache and MXFP4 KV cache (--index-cache-dtype fp4, --kv-cache-dtype fp4) - #2962
FP4 index cache, --index-cache-dtype fp4: the indexer's key cache is stored as MXFP4 pages (e2m1 data, one e8m0 scale per 32 elements, 8704 B a 128-token page), written by aiter's fused QK-norm / rope / cache writer, scored by aiter's FlyDSL scorers (an fp8 index query, e4m3), then selected by the same Triton selector as the other index caches or, on wide rows, aiter's per-row selector. This one can be used alone: the K/V cache stays bf16 or fp8.
FP4 KV cache, --kv-cache-dtype fp4: K and V are MXFP4 pages of the same format, attended by sparse_pa_mxfp4 (aiter's FlyDSL pa_decode with MXFP4 K/V, an e4m3 query, one row per (token, KV head)). It needs the FP4 index cache too, because the attention reads exactly the rows the selection wrote and only the MXFP4 index scorer is sized to those rows.
Scope: MiniMax-M3.1 (every attention layer an indexed sparse one). Plain MiniMax-M3 (a checkpoint with dense attention layers) is not supported for FP4 KV and is refused at startup (see the table); DeepSeek-V4.1's own FP4 KV is unchanged, and DeepSeek-V4 and every other model refuse --kv-cache-dtype fp4. Not in this PR: NVFP4, context parallelism over the MXFP4 caches (refused), KV offload / P-D transfer of MXFP4 pages (refused), the vLLM/SGLang bridges (refused), speculative drafts with FP4 KV (a draft with a pool of its own is bf16; a draft in the target's pool is refused).
--spec-decode-kv-cache-dtype is NOT part of this PR: it was split out on review into #3235 (see "Related PRs"). What an FP4 target needs of that code stays here.
What is accepted and what is refused at startup
Rules live in atom/models/minimax_m3/config.py (validate_runtime_config) and docs/configuration_guide.md, and nowhere else. Nothing falls back to FP8.
--kv-cache-dtype
--index-cache-dtype
result on MiniMax-M3(.1)
bf16 / fp8
omitted, bf16 or fp8
as before this PR
bf16 / fp8
fp4
accepted: MXFP4 index only
fp4
fp4
accepted, if all of the conditions below hold
fp4
omitted
accepted: the index defaults to fp4 (follows the KV dtype)
Either cache set to fp4 also requires: native ATOM (refused under the vLLM/SGLang bridges), --block-size 128, a gfx950 GPU (read from the live device, not from GPU_ARCHS). --kv-cache-dtype fp4 further requires: sparse_attention_config with sparse_attention_freq exactly 1 for each of num_hidden_layers (so MiniMax-M3's original checkpoints with dense attention layers are refused), a bf16 model, no KV offload and no --kv-transfer-config, and no speculative draft left with FP4 KV. Context parallelism over either MXFP4 cache is refused by indexer_cp_unsupported_reason.
Other models: _check_fp4_kv in Config.__post_init__ allows --kv-cache-dtype fp4 for the MiniMax-M3 architecture and DeepSeek-V4.1 only (message: "MiniMax-M3.1 and DeepSeek-V4.1 only"; the M3 hook then refuses what is not M3.1); every other model is refused.
Files
Config and CLI: atom/config.py, atom/model_engine/arg_utils.py, atom/models/minimax_m3/config.py (the new hook), docs/configuration_guide.md.
M3 kernels and metadata: atom/model_ops/minimax_m3/geometry.py, mxfp4_pages.py (page layout), index_buffers.py (the scorer's workspace and work map, one record per prefix), index_topk.py (scorers, selection, block-table emission), sparse_attn.py, sparse_attn_mxfp4.py.
Attention and pool: attention_mha.py (writer, KV/index paths, the row rule _metadata_rows), attentions/aiter_attention.py (index cache spec, scorer allocation, TBO), attentions/mha_kv_pool.py and pool_layout/pool_rows.py (KvLayout: SHUFFLE or MXFP4 pages), base_attention.py (plan_segment in the plan key), backends.py.
Drafts (what an fp4 target needs): spec_decode/draft_kv.py, utils/selector.py (DRAFT_POOL_FAMILIES), the DSpark / EAGLE proposers and the calibration identity read SpeculativeConfig.draft_kv_dtype; DeepSeek-V4's fp8 -> bf16 KV fallback is decided in Config (it was in the V4 builder) so the draft's dtype is settled after it.
Tests: new tests/test_minimax_m3_mxfp4_{cache,index,attention,write,decode_batch}.py, test_minimax_m3_flydsl_optional.py, test_minimax_m3_emit_pad_row.py, (the index-cache-spec test file was dropped in the trim); changed: test_minimax_m3_index_topk.py, test_draft_kv_builder.py, test_dspark*.py, test_indexer_cp_gate.py, test_kv_pool_contract.py, test_v4_indexer_defaults.py, test_arg_utils_spec.py, lmcache/pp-pd stubs (their fake builders follow the new index-cache spec).
Net diff against main: see the PR header (head 880044c46); the tests this PR adds were halved in the last push (one test per behaviour).
Other behaviour changes in this PR (outside the MXFP4 paths)
Pad-row fix (B12):_emit_sparse_block_table_row clamps a row's causal length where it enters, so a pad row (length 0, q > 1) computes nothing from a negative length. The helper is shared by every index cache (bf16/fp8 Triton, MXFP4, candidate exchange); live rows are unchanged.
bf16 / fp8 index on aiter's per-row selector: with the import path now aiter.ops.flydsl.topk.topk_per_row_small_k (main's flat import path does not exist in a current aiter, so main ran Triton there), a bf16 / fp8 row of 131072+ tokens is served by aiter's selector, not Triton. Not measured for bf16 / fp8. An aiter without that module fails at that row (no fallback; see Dependencies).
V4: DeepSeek-V4's fp8 KV falls back to bf16 off gfx950 / gfx1250 in Config, not in the metadata builder.
compile hash: the speculative tuple gains the draft dtype, so every speculative model's compile cache key changes once.
Dependencies
Minimum aiter (documented in docs/configuration_guide.md, no code probe): a build with ROCm/aiter #7014 (topk_per_row_small_k, used by every M3 index cache on rows of 131072+ tokens), #7057 (fused writer into MXFP4 pages), #7060 and #7061 (MXFP4 index decode / prefill scorers), #7056 (MXFP4 K/V in pa_decode). Upgrade ATOM and aiter together. No new aiter API beyond these is required.
What was run on a GPU, and at which head: the last server-level run was at 55070fb73; nothing server-level was run on this head, and 70+ commits and a merge of main came after it.
At 55070fb73 (the flag then spelled --kv-cache-dtype mxfp4 --index-cache-dtype mxfp4; the fp4 spelling came later): TP4, DSpark with 7 draft tokens, MXFP4 KV and index: 279 GPU tests passed, 32 skipped; GSM8K (all 1319 questions) flexible 0.9431 +/- 0.0064, strict 0.8613 +/- 0.0095, 12 empty responses; TP2 --enable-tbo all smoke on 300 questions, flexible 0.9633, strict 0.9200. Earlier heads (13f413d0b, bb1719ea9): GSM8K 0.9416 / 0.8825 and 0.9469 / 0.8704 (TP4), padded-replay and TBO smokes (TP2), AgentX c32 368.92 tok/s/GPU on 13f413d0b; these are older code and are not evidence for this head.
After 55070fb73: kernel-level checks on one MI355X only (bit-equal selections with and without the unfilled workspace out=, a run_perftest table of scorer + selector, contiguous vs aiter's layout), at aiter 5469555096, posted in the PR comments; not re-run at this head.
On the CPU (container with torch and aiter importable, no GPU), the subset of the touched tests plus test_config*, test_spec*, test_mtp_*, test_lmcache*: 20 failed / 1798 passed / 209 skipped / 2 collection errors (head 880044c46). The 20 failures and 2 collection errors are the same ids as on the unmodified head, except that the FlyDSL-optional test was merged into one subprocess test, which fails in that image for the same missing aiter module as the old one it replaces (missing aiter modules in that image: test_spec_lossless_staging, test_minimax_m3_flydsl_optional, ...). The gfx950-gated tests (the real-kernel writer, attention, index and decode-batch tests, and the compiled variant of test_minimax_m3_emit_pad_row.py) are skipped there and have not run at this head.
CI at the last check: Black, Ruff, non-GPU unit tests, pre-checkin pass; the accuracy jobs were pending, and the MiniMax-M3-MXFP4 accuracy job uses --kv_cache_dtype fp8, so a green CI says nothing about the fp4 paths.
Not validated on any GPU at this head: the FP4 KV and index end to end; TBO decode output parity and split-bucket graph replay; a DP group with one rank prefilling while another decodes; an MHA EAGLE3 / DSpark draft under an fp4 target; V4's fp8 -> bf16 fallback on gfx942; the long-context (131072+) bf16 / fp8 rows on aiter's selector; the aiter fused-writer change (one name table, B15).
Known limits
The scorer works at the model-length width on every step (eager, graph, TBO ubatch): its grid and the score workspace follow max_model_len, not the batch's context (B1, B3). The workspace is H x max(max_bs*max_qlen, max_num_batched_tokens) x cdiv(max_model_len, 128) x 4 B (512 MiB at TP4 and 1M context, 4x at TP1), allocated at builder construction, so the memory profile counts it.
Decode attention over MXFP4 pages runs in plan segments (_FLYDSL_PLAN_MAX_BATCH) and has no gluon fallback; eager decode plans accumulate per distinct row count in the process-wide plan cache (B4, B5).
MhaKvPool still takes both kv_dtype and a KvLayout (B7, B8); the shared builder and the attention impl both decide "M3 sparse => MXFP4"; the per-model fp4 allow-list sits in config.py (B13); two skip-index writers exist (Triton for a non-fp4 KV skip layer, aiter's skip_index_branch for fp4 KV).
The review threads on these are answered one by one in the PR; none is closed by this description.
Follow-ups
Deferred with the maintainer's agreement, one issue each:
ATOM_M3_SPARSE_KV_MXFP4=1 with --kv_cache_dtype fp8 --index-cache-dtype mxfp4
declares the MXFP4 caches the aiter kernels read and write: one 8704 B field
(128 x 64 B e2m1 data, 128 x 4 B e8m0 scales) per (page, kv head) for K and for
V, and a single 8704 B field per block for the index key (136 + 68 B a token).
The sparse layers get a pool of their own: KvGeometry and MhaKvPool gain a
kv_format field ("" keeps every existing pool unchanged), so the MXFP4 pages sit
beside any dense layers' SHUFFLE pool.
Startup refuses plugin mode, a block size other than 128 and non-gfx950. Mono
declines MXFP4 KV, and indexer CP falls back with a warning. This commit only
declares the pool and the switches; the write and read paths follow.
Co-Authored-By: Claude Code <noreply@anthropic.com>
… index key
rope_cache with the MXFP4 switches calls fused_qknorm_idxrqknorm with
kv_cache_dtype="mxfp4" straight onto the pool pages (no staging buffer and no
separate store kernel). The main q comes back as e4m3: one float8_e4m3fn q_out
is passed and the kernel writes the unit-scale saturating cast with no bf16 q
row (the final aiter API, ROCm/aiter#7057). A layer that reuses another layer's
top-k writes K and V only and passes skip_index_branch.
Adds dequantize_pages, the oracle the write test compares the cache against.
Needs aiter's fused_qknorm_idxrqknorm with kv_cache_dtype="mxfp4"
(ROCm/aiter#7057, merged).
Co-Authored-By: Claude Code <noreply@anthropic.com>
…s; page-128 block table
index_topk's decode and prefill take index_mxfp4: the scores come from aiter's
score_flydsl_fp4 / score_prefill_flydsl_fp4 over the two views (data and scale)
of the pool page, in the layout the Triton scorers produce, so the selection
and the emit after them are unchanged. pages_per_block is a parameter of the
emit now (8 for the page-16 SHUFFLE cache, 1 for page-128 caches), which makes
the block table one page a block for the MXFP4 KV.
The decode scorer's work map is built once per step into a persistent buffer
(capacity over every step width, bound = max_model_len / 128) by
make_sparse_decode_metadata, outside the graph. The decode scorer's fp32 score
buffer is persistent too: without an out the aiter scorer runs a torch.full(-inf)
in every layer. The metadata refills the buffer's view once per step and every
layer passes it as out. Where the scorer lays scores out transposed (S*H >= 16,
not TP4) the view is None and nothing changes.
Tests: scorers against a dequantised oracle; replay of a captured decode against
a refilled work map; the persistent score buffer equals a fresh -inf buffer bit
for bit over permuted, shortened and smaller steps. Needs aiter's
score_flydsl_fp4 and score_prefill_flydsl_fp4 (ROCm/aiter#7060, #7061, merged).
Co-Authored-By: Claude Code <noreply@anthropic.com>
…DEX_Q_FP8
sparse_pa_mxfp4 folds (token, kv head) into rows, hands the e4m3 q and the pool's
K and V pages (data and scale views of one 8704 B page) to aiter's pa_decode and
bypasses run_pa_decode's fp8 and ASM gates. attention_mha routes the sparse
prefill and decode through it, and asks the selection for a page-128 block table
(one page a block) and the MXFP4 scorer, with the step's work map and score
buffer on decode.
Decode keeps the planned path (the captured shapes are fixed): rows past the
planner's batch limit run in segments, each with a plan of its own, refreshed
once a step as the tail page is always selected. Prefill is plan-free
(work_plan=None): plan_pa_decode JIT-compiles a kernel specialised on the batch
size for every row count not seen before, and a prefill chunk has a new row
count almost every forward. One task and one direct write per row means one JIT
specialisation whatever the rows.
ATOM_M3_INDEX_Q_FP8 (off by default, refused without --index-cache-dtype mxfp4)
has the fused writer emit the indexer query as e4m3 and the scorer read it as
such.
Tests: decode and prefill against the dequantised oracle (Hkv 1 and 2); any row
count 1..700; plan-free bit for bit the planned kernel with a one-partition
plan; no new planner or PA kernel build across 12 new row counts.
Needs aiter's pa_decode with MXFP4 K/V (ROCm/aiter#7056, merged).
Co-Authored-By: Claude Code <noreply@anthropic.com>
Runs automatically on every eligible PR before approval:
✅ Pre Checkin: Black, Ruff, catalog schema validation, non-GPU unit tests
Heavy model tests:
✅ Run after the PR is approved and Pre Checkin passes
✅ Run immediately when an approval review is submitted
✅ Can be requested before approval with labels
Label
Tests
ci:full
Run all heavy PR model tests: native ATOM, vLLM, and SGLang
ci:atom
Run native ATOM model accuracy tests
ci:vllm
Run ATOM vLLM OOT model accuracy tests
ci:sglang
Run ATOM SGLang model accuracy tests
Heavy jobs are skipped when the PR is not approved and no matching ci:* label is present.
Add labels via the sidebar or gh pr edit 2962 --add-label <label>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Padded MXFP4 replay can supply stale, batch-incompatible metadata, the new CPU test fails on an unmocked aiter import, and FP8 cache reuse and noncontiguous score handling have runtime failures.
Adds opt-in MXFP4 sparse KV and index caches for MiniMax-M3 on gfx950, integrating AITER’s fused cache writer, index scorers, and paged attention while preserving existing defaults.
Changes:
Adds cache configuration, startup checks, and separate pool geometry for MXFP4 pages.
The reason will be displayed to describe this comment to others. Learn more.
some changes needed..
any GPU run of the mxfp4 path? gsm8k and e2e perf for this config?
ci Run non-GPU unit tests failed — looks like the pr itself (medium): Log was not captured (0 bytes), so this is inferred: the job installs CPU torch only and runs pytest tests/, and the new tests/test_minimax_m3_mxfp4_cache.py has no aiter guard -- test_an_fp8_index_q_needs_the_mxfp4_index reaches from aiter.jit.utils.chip_info import get_gfx in _check_m3_mxfp4 (atom/config.py:1947) and raises ModuleNotFoundError.
unit-test workflow: pip3 install torch --index-url https://download.pytorch.org/whl/cpu ... pip3 install pytest msgpack quart requests
run_unit_tests.sh: python -m pytest tests/ --ignore=tests/plugin --ignore=tests/entrypoints/test_openai_server.py
atom/config.py:1947: from aiter.jit.utils.chip_info import get_gfx
next: Stub get_gfx in the test (or importorskip aiter, as run_unit_tests.sh documents for GPU-only modules), then re-run the job; same job passes on main at 22:07. https://github.com/ROCm/ATOM/actions/runs/41757321369/job/126336381142
…use key, TP2 scores
Build the MXFP4 work map and score view from the captured batch on a padded graph
replay; key the pa_decode plan by TBO ubatch; record the FP8 index-query dtype in
the top-k reuse key; leave a transposed score to the Triton selector; check
sparse_local_block at startup; stub the arch query in the CPU cache test; document
the two env vars; drop the unused require_supported and the personal-branch hint.
Co-Authored-By: Claude Code <noreply@anthropic.com>
… padded replay
n_valid_column_per_row is head-major, so with more than one index head its head
offsets depend on the batch the graph was captured at, like the work map and the
score view. Build it from the padded lengths when they are given (MXFP4 index only).
Co-Authored-By: Claude Code <noreply@anthropic.com>
Round 2 pushed 13f413d. GSM8K: no regression within sampling variance. Performance follows in a separate comment.
Limit: the fixed score was measured on 2e700ec, not on 13f413d. The last commit only changes selector inputs the current aiter does not use (Triton fallback), so no change is expected, but it is not rerun end to end. GPU tests for padded graph replay, FP8-query top-k reuse and TBO plan keys are also not run yet.
Evidence
MiniMax-M3.1, TP4 (DSpark7), MXFP4 KV and index cache, block size 128, ATOM_M3_SPARSE_KV_MXFP4=1 --kv_cache_dtype fp8 --index-cache-dtype mxfp4, real acceptance. GSM8K, concurrency 32, 1319 questions, 5-shot, chat template, multi-turn, temperature 0, max new tokens 4096. Same machine (GPU 0-3), same aiter (5469555096).
Differences are within sampling variance; per-question texts are nondeterministic between runs. Reference from #2770 on a different machine, image and aiter: 0.9424 / 0.8711.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Unresolved MXFP4 issues can cause padded decode failures, incorrectly validate GPU support, and bypass once-per-step work-map and score-buffer reuse during TBO decode.
No open issues 🎉
3 previously missed issues
In code that hasn't changed since last review
Use the runtime GPU architecture for the capability check
atom/config.py:1958
get_gfx() honors the compilation-target override GPU_ARCHS, so this startup check can accept unsupported hardware or reject a supported gfx950 device. Query get_gfx_runtime() once and use that value for both the capability check and error message. Update the architecture mock in tests/test_minimax_m3_mxfp4_cache.py to mock the runtime query too.
Provide MXFP4 index buffers for TBO microbatches or reject the combination
atom/model_ops/attention_mha.py:1651
With MXFP4 index caching and TBO decode, build_ubatch_metadata() omits the persistent work map and score buffer, so these fields are None and each sparse layer rebuilds the map and allocates/fills scores. The full-batch buffers prepared in prepare_decode() are unused by those microbatches, losing the once-per-step optimization. Provide independently refreshed per-microbatch buffers, or explicitly reject MXFP4 index caching with TBO decode until that path is supported.
Pad decode lengths and block tables to match the eager query batch
On padded eager decode, including attention outside a TR_HYBRID token graph, the MXFP4 query, work map and score buffer use the padded batch, but decode.seq_lens and decode.block_table still use scheduled_bs. For 3 requests padded to 8, aiter derives batch=8 from the query and rejects the 3-row table with ValueError("block_table: invalid shape or layout"). Publish matching padded decode lengths and block tables for the scorer and selector, and add a regression through prepare_decode that executes attention with a padded query.
Round 2 pushed 13f413d; performance below is for the first-repair tree, not 13f413d. AgentX c32 full 1200 s run, 0 errors. Final-head GPU tests and rerun are ongoing.
Measured tree b20dec0f4a24 (same tree as 2e700ec): 367.72 output tok/s/GPU (1470.88 / 4), 90th-percentile interactivity 108.04 tok/s/user (1000 / ITL p90 9.256 ms). aiter 5469555096; top-k selection falls back to Triton. #2770 reported 371.49 / 109.97 on a different machine, image and aiter, so it is a reference, not an A/B.
Round 3 pushed 13f413d; this is the final-head accuracy and GPU validation. GSM8K matches the earlier trees within sampling variance; GPU tests pass; padded graph replay ran on the real server. Final-head performance is still running and is not reported here.
Limits: TP2 / 2-index-head real replay is untested. The aiter pinned here has no small-k selector, so the new selector bounds ran through the Triton fallback and the aiter selector path is not exercised (its test is skipped). TBO was covered only by the unit test, not a real --enable-tbo server run. The padded-replay metadata tests are unit tests and do not themselves run a graph replay; the server trace below is the replay evidence.
Evidence
Final head 13f413d (tree d8e1c459c283), aiter 5469555096, same config as the earlier GSM8K run: TP4 on GPU 0-3, MXFP4 KV and index cache, block size 128, concurrency 32, 1319 questions, 5-shot, chat template, temperature 0, max new tokens 4096, real acceptance.
GPU pytest over the M3 MXFP4 and index top-k test files: 213 passed, 33 skipped, 0 failed. Skips: the aiter small-k selector is not installed (1), gfx1250 launch-policy cases (32). Includes the padded-replay, FP8-query reuse, TBO plan-key and contiguous-score tests.
Rank 0 Kineto trace of the server during the GSM8K run: 958 decode steps, 670 at full batch 32 and 288 padded (batch 29/32 six steps, 30/32 54, 31/32 228), each with propose_dspark on the captured graph. TP4 has one index head. The run finished with the accuracy above and no error.
Round 3 pushed 13f413d; final-head performance: 368.92 output tok/s/GPU, 106.78 tok/s/user at 90th-percentile ITL, 0 errors. Same config as the first-repair run (367.72 / 108.04); #2770's 371.49 / 109.97 is a different machine, image and aiter, so neither is a strict A/B.
ATOM_M3_INDEX_Q_FP8 is off (default) in this run, so FP8 index-query reuse is not measured. The FP8 main Q read by the PA kernel is a separate path and is no evidence for it.
Evidence
Tree d8e1c459c283, aiter 5469555096, top-k selection through the Triton fallback; TP4 (DSpark7, GPU 0-3), MXFP4 KV and index cache, block size 128, ATOM_M3_SPARSE_KV_MXFP4=1 --kv_cache_dtype fp8 --index-cache-dtype mxfp4.
AIPerf 1.0.12, AgentX semianalysis_cc_traces_weka_062126, concurrency 32, 1200 s profiling window (client duration 1222.8 s with drain), warmup first; synthetic MTP acceptance, 4.04 tokens per forward.
1902 requests, 0 errors, not cancelled, submission_valid true; server log has no error.
Output 1475.66 tok/s total (/ 4 = 368.92 per GPU). ITL p90 9.365 ms (1000 / 9.365 = 106.78); TTFT p50 317 ms, p90 637 ms.
Round 3 pushed 13f413d; reply to @valar-cop's review. Final-head GSM8K (accuracy) and AgentX c32 (performance) are posted. A real TP4 padded replay ran, but with the Triton selector: the pinned aiter has no small-k selector, so a padded replay with the aiter selector serving is not run. The three new inline threads are answered there; none is fixed in this head.
Round 3 pushed 13f413d; same-machine A/B for @valar-cop: output throughput -0.23%, p90 interactivity -0.83% against the pre-fix tree, within run-to-run spread. No code change.
AgentX c32, AIPerf 1.0.12, 1200 s profiling window, 1902 requests and 0 errors in each run; same container and GPU 0-3, TP4, MXFP4 KV and index cache, block size 128, aiter 5469555096, synthetic acceptance 4.04 tokens per forward.
Output totals 1479.07 and 1475.66 tok/s; client durations 1221.0 s and 1222.8 s.
Earlier first-repair run (2e700ec): 367.72 / 108.04, comment.
Round 3 pushed 13f413d; reply to @valar-cop's 05:01 review. A real TP2 / 2-index-head padded replay ran and GSM8K held. The selector-serving replay is not done: the small-k selector still falls back to Triton, so that run is pending. No fix is claimed here for the two new inline threads.
Evidence
13f413d, TP2 on GPU 0-1, no speculative decoding (DSpark needs TP4 or more; the TP2 DSpark launch is refused at startup), MXFP4 KV and index cache, block size 128, CUDA graphs on.
Rank 0 trace: 1654 decode steps, 202 padded graph steps (31/32 184, 30/32 18), scored by m3_index_score_S1_H2_fp4 (2 index heads).
GSM8K, first 300 of 1319 questions, concurrency 32, 5-shot: flexible-extract 0.9567 ± 0.0118, strict-match 0.9300 ± 0.0148. This is a 300-question smoke, not comparable to the full-set numbers.
@valarLip re-read at da0c003; head is now 2f174c2 (fast-forward, 9 commits). Description and title are rewritten for this head. Nothing below was run on a GPU in this round (all cards were reserved); what is still open is said per item.
Your six blocking items (11-12)
aiter version probe: removed in 224ae84, the import is direct. The repo records no aiter minimum anywhere, so 2995ae4 / b60961e add a "Minimum aiter" note to docs/configuration_guide.md (#7014 the per-row selector, #7057 the fused writer, #7060 / #7061 the MXFP4 index scorers, #7056 MXFP4 pa_decode). Consequence, plainly: on an aiter without #7014 a bf16 / fp8 row of 131072+ tokens raises ImportError, where main fell back to Triton (Copilot flagged exactly this). I kept the direct import, as your rule says; the fallback would be one try/except, say if you want it.
One row rule: _metadata_rows serves prefill and decode (224ae84).
Boundaries: scorer allocation is in minimax_m3/index_buffers.py; _index_work_map_blocks, the index_mxfp4 bool and the kwargs helpers are gone (224ae84). Imports: nothing is imported per layer any more. The remaining function-local imports run at construction or startup, plus two cached accessors (flydsl_ops, _small_k_selector) that keep FlyDSL optional for bf16 / fp8 and index_buffers.py importable without aiter.
Title and description: rewritten (M3.1; FP4 index and FP4 KV both in scope; plain M3 with dense layers refused; the GPU validation says the last server-level run was at 55070fb).
Split: --spec-decode-kv-cache-dtype, its EngineArgs plumbing, the requested value, the "asked" half of resolve_draft_kv_dtype, the offload refusal on a differing draft dtype, its docs row and tests are in [Spec decode] --spec-decode-kv-cache-dtype: bf16/fp8 KV for a draft that has its own pool #3235 (based on main, independent of this PR, requested and settled values as two fields as you asked in 16aa94dupdate the link to the repo #3 / 97f7aacgpt_oss update: add fused_qk_rope_reshape_and_cache #9). They are gone from this PR (212f185). What stays here is what an fp4 target needs: a draft with a pool of its own is bf16 under --kv-cache-dtype fp4 (an fp4 draft has no layout), the family table that says whether a draft has its own pool, V4's fp8 -> bf16 fallback ordering in Config, and the one settled field draft_kv_dtype. resolve_draft_kv_dtype and the EAGLE / DSpark builder changes are the same code in both PRs, so they can leave this one only if [Spec decode] --spec-decode-kv-cache-dtype: bf16/fp8 KV for a draft that has its own pool #3235 lands first. Order: the two are independent; whichever lands second is rebased. This PR is 49 files, +4198 / -402 (was +4284 / -402): the split alone does not make it small.
Also changed
B12: c48f677 (thread answered). B15, the mapping half: 70bd1ad.
11-10 update readme for deepseek #6: an omitted --index-cache-dtype beside --kv-cache-dtype fp4 now defaults to fp4; an explicit bf16 / fp8 is still refused (2f174c2, test test_an_fp4_kv_without_an_index_flag_gets_the_fp4_index). This reverses 4e4dca9.
B13: the allow-list is still in config.py; its message and docstring now say MiniMax-M3.1 (2f174c2). Moving it needs a per-model hook for the generic refusal: not done.
Three small commits (75ffc24, 0c514f9, 2e3790a): two duplicate tests, a one-line wrapper, a long docstring removed.
Still open, not changed
B1, B3, B4: the scorer works at the model-length width on every step (eager, graph, TBO ubatch); eager decode plans still accumulate per row count. Needs a measurement and, for B3, an aiter change.
B5: plan_segment and the segment loop stay; they need MXFP4 scale support in run_pa_decode.
B7, B8, the KvLayout half: the pool still takes kv_dtype and a layout; about 80 lines and 20 test sites, and main's transfer_pages reads pool.kv_dtype.
The two skip-index writers; the DP test (one rank prefilling while another decodes); T9, the score workspace (512 MiB at TP4 / 1M context, 4x at TP1).
CI: the MiniMax-M3-MXFP4 accuracy job runs --kv_cache_dtype fp8, so a green CI says nothing about the fp4 paths. An fp4 job needs a GPU-runner decision from the maintainers; I added none.
GPU validation of this head: the last server-level run was 55070fb (with the mxfp4 spelling of the flag); since then only kernel-level checks on one card. The rerun commands are ready.
Seven threads (B1, B3, B4, B5, B7, B8, B13) show as resolved because we resolved them with "tracked for follow-up". That was premature: they are open work, and I answered each below; I did not reopen them.
Corrections to things I said earlier (the threads are not edited)
6647558639 (11-05): "an eager main decode step scores at the batch's block count" is false; every step scores at cdiv(max_model_len, 128) (the shrink was removed in 7bbdd07). And "tests/test_minimax_m3_skip_index_writer.py is written" is false: there is no such file.
T28 (B8): "MhaKvPool no longer stores kv_dtype" is false at this head: the merge of main restored self.kv_dtype, which main's transfer_pages reads.
5771103020 item 10: "tests/test_v41_geometry_parity.py reads that spelling via AST": there is no such file and no test reads it. model_type == "deepseek_v41_text" is main's own spelling in five places; the PR adds one. Nothing changed.
T26 (B6): the reply names decode_rows_batch, inlined later (97f7aac).
6731087765 item 2: "same rule for every index cache" was superseded; the rule is MXFP4-only and fp4 KV with another index is refused.
T22 (B2), T30 (B10), T31 (B11), T39, T46 / T52: the threads say "follow-up / not changed / still open", but the code was changed later (620692b, fd64ce3, 785e40f, 68fc39c, 4cc42e7).
Reply 6786465138 item 3: Config also imports aiter for V4's fp8 -> bf16 fallback (get_gfx), not only for M3.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because FP4 cache configurations permit unsupported context sharding, creating a critical mismatch between rank-local KV storage and global sparse-scoring context lengths.
…uplicate tests
Roughly halves the test code the PR adds. Merged near-duplicates into parametrized
cases (padded eager / graph / workspace rows, the startup refusals as one table, the
row rule for decode, prefill and a split TBO step), and removed tests that only
asserted mocks, private-helper plumbing or a second copy of another test
(test_aiter_index_cache_spec.py, the draft-pool agreement tests, a repr pin, ...).
Every deleted or merged test is listed with the review thread that asked for it in
TRIM_*.txt, and the thread is told. The B12 pad-row test, the FlyDSL-optional test
(now one subprocess test), the refusals, the row rules and the real-kernel
(gfx950, GPU-pending) tests each keep a case.
Co-Authored-By: Claude <noreply@anthropic.com>
Comment and docstring trims only in index_topk, index_buffers, sparse_attn_mxfp4,
sparse_attn, attention_mha, aiter_attention, models/minimax_m3/config, draft_kv and
config; comments that a review thread asked for are kept.
Co-Authored-By: Claude <noreply@anthropic.com>
…hat build a pool
It guards a silent failure (a backend with make_kv_pool missing from DRAFT_POOL_FAMILIES
leaves its draft sharing the target's rows); it replaced main's flag-agreement test.
Co-Authored-By: Claude <noreply@anthropic.com>
M3 has no DCP gather/merge: with DCP > 1 the writer stores only a rank's own
tokens while the sparse readers take global lengths, so a read can reach rows
nobody wrote. validate_runtime_config now refuses decode_context_parallel_size > 1
when either MXFP4 cache is on; tests cover the FP4-KV and index-only cases.
Co-Authored-By: Claude <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because interactions between FP4 KV caching, DSpark draft dtype selection, and sequence-parallel top-k reuse can cause startup rejection or fused-writer shape failures.
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Action required before merge (4)
Run the fp4 paths on a GPU at this head. The last server-level run was at 55070fb73, more than 70 commits and a merge of main ago. Every gfx950 test (writer, attention, index, decode-batch) is skipped in CI, and the MiniMax-M3-MXFP4 accuracy job uses --kv_cache_dtype fp8. Please post, at 2808171728: GSM8K for M3.1 TP4 + DSpark with --kv-cache-dtype fp4 (index fp4); an --enable-tbo all smoke; the gfx950 pytest files; and Copilot's EAGLE3 draft-dtype test, which I asked for on 11-12.
Land #3235 first, then rebase so this PR keeps only the fp4 -> bf16 draft rule.resolve_draft_kv_dtype (config.py:1959), draft_owns_kv_pool / DRAFT_POOL_METHODS (spec_decode/draft_kv.py:17-44), DRAFT_POOL_FAMILIES (utils/selector.py:77), SpeculativeConfig.draft_kv_dtype, and the DSpark/EAGLE/calibration edits are the same code in both PRs (your 11-12 reply says so). Two open copies of one mechanism will drift. My 11-12 item 6 is only half done.
Close the design items still open from earlier rounds; my position is unchanged.
B5: the segment loop in minimax_m3/sparse_attn_mxfp4.py:108, plus plan_segment added to _flydsl_prepare_explicit_plan (base_attention.py:502).
B7/B8, the KvLayout half: MhaKvPool still takes both kv_dtype and layout (mha_kv_pool.py:204-208), and transfer_pages reports self.kv_dtype (:379).
KvGeometry.layout is still typed object | None (pool_rows.py:38).
The shared MHA builder still recognises M3 and imports Mxfp4KvLayout (aiter_attention.py:419-427).
B13: the per-model fp4 allow-list _check_fp4_kv is still in config.py:1948.
The two skip-index writers remain.
Resolving those threads as "tracked for follow-up" does not close them. Either fix them here, or get an explicit maintainer OK to defer each one with a linked issue.
Document the refusals this head added. The kv_cache_dtype / index_cache_dtype rows in docs/configuration_guide.md:42-43 don't mention two refusals:
decode context parallelism (models/minimax_m3/config.py:69, from 280817172);
an MTP draft left with fp4 KV.
The description says the rules live "in config.py and the doc and nowhere else".
Re-reviewed at 2808171. Already fixed: aiter try/except ImportError probe (direct import, aiter #7014/#7056/#7057/#7060/#7061 all merged); getattr(config, "speculative_config", None) test-stub shim; one row rule _metadata_rows for prefill and decode; scorer allocation moved to minimax_m3/index_buffers.py with _index_work_map_blocks / index_mxfp4 / kwargs helpers removed; title and description; B12 pad-row clamp; B14; fp4 KV defaults the index to fp4; DCP refusal (Copilot). Non-GPU unit tests, Ruff and Black pass.
…cument the DCP and in-pool-draft refusals
The shared MHA builder no longer imports the MXFP4 layout or tests the
dtype: SparseMHAPagedAttentionImpl.kv_layout says how its pages are laid out.
KvGeometry.layout is a KvLayout, not object. The configuration guide now lists
the decode-context-parallel and in-pool-draft refusals, and three comments that
still described the removed backend flag are corrected.
Co-Authored-By: Claude <noreply@anthropic.com>
… passes
_run built the buffers even when the caller handed it the metadata, so the
captured variant of the padded-step index test allocated inside the capture
(hipErrorStreamCaptureUnsupported on gfx950).
Co-Authored-By: Claude <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because an unpublished critical finding identifies missing index columns in the FP4 KV/Ulysses writer path that cause inference to fail at the first top-k reuse layer.
@valarLip re-read of your 11-12 11:22 review (5873510797) at 2808171. New head: 015f202, pushed once. It carries the changes below; the unpushed test fix is item 1.
1. fp4 paths on a GPU at this head
Run on 4x MI355X (gfx950), the code of 015f202 (plus one test-only commit, below), aiter from the rocm/atom-dev:nightly_202611041513 image (commit 627461276).
gfx950 pytest files (test_minimax_m3_emit_pad_row, _index_topk, _mxfp4_index, _mxfp4_write, _mxfp4_attention, _mxfp4_decode_batch): 187 passed, 32 skipped, 1 failed. The 32 skips are the gfx1250 launch-policy regression parametrisations in test_minimax_m3_index_topk.py, as intended on gfx950. The failure is a bug in the test, not in the product: _run in test_minimax_m3_mxfp4_index.py allocated the scorer buffers inside a CUDA-graph capture (hipErrorStreamCaptureUnsupported) in the graph variant of the padded-step test. Fixed in 7fe048d (test only, +3/-2); that file then gives 10 passed. That commit is not pushed yet (I pushed once this round); it goes with the next push.
GSM8K, M3.1 TP4, --kv_cache_dtype fp4 --index-cache-dtype fp4, DSpark with 7 drafts, --block-size 128, 5-shot chat, temperature 0, first 200 problems only: flexible 0.960 (stderr 0.014), strict 0.900 (0.021). Startup log: 60 h1d128fp4 rows in the target pool, the DSpark draft pool allocated separately. Cumulative [MTP Stats] after the run: 4.52 tokens per forward, acceptance 50.35%. It is 200 of 1319 questions, and the server was launched without the recipe's --default-chat-template-kwargs thinking flag, so it is not like-for-like with the recipe's 0.955 / 0.914 (fp8 KV, full set).
--enable-tbo all smoke, fp4 KV + fp4 index, without DSpark. With DSpark the server refuses to start: NotImplementedError: MiniMax M3 sparse TBO decode with max_q_len > 1 is not implemented (aiter_attention.py, the same check as on main). Without it the server starts (TBO enabled: model wrapped with UBatchWrapper), captures graphs and serves. GSM8K, first 100 problems: flexible 0.93 / strict 0.88. The same checkout without TBO, two runs: 0.96 / 0.93 and 0.95 / 0.92. On strict outcomes the TBO run differs from the two non-TBO runs in 11 and 6 of 100 problems, the two non-TBO runs from each other in 9; raw text is identical in only 13 to 16 of 100 in every pair. So the TBO result is inside the run-to-run spread at N=100; that is a smoke, not a parity proof. Split-bucket graph replay under DP is not covered.
Copilot's EAGLE3 draft-dtype test (test_an_eagle3_draft_attention_is_built_at_the_dtype_of_its_pool): passes. It stubs EagleProposer, imports aiter, and allocates no pool and runs no forward, so it needs no GPU; I ran it on a CPU host.
Observation: every fp4 server start here printed KV cache allocation mismatch: expected=169GB, actual=136GB (16-20%), with and without DSpark and TBO; I did not run bf16 or fp8 for comparison, so I do not know whether it is specific to fp4.
Not run: the other cache combinations (fp8 KV + fp4 index, bf16 index), a DP step with one rank prefilling while another decodes, a request of 131072 tokens or more.
2. Land #3235 first, then rebase
This branch is already the flag-less form of #3235: no --spec-decode-kv-cache-dtype, no "asked" field, and resolve_draft_kv_dtype(target_dtype, *, own_pool) is the one-line fp4 -> bf16 rule. What is still the same code is the mechanism (draft_owns_kv_pool, DRAFT_POOL_FAMILIES, SpeculativeConfig.draft_kv_dtype and the proposer, calibration and hash edits). I cannot rebase without a force push, so my plan is: when #3235 merges, merge main into this branch (a merge commit), take main's side in the draft files and re-add the fp4 rule on top of main's resolve_draft_kv_dtype. I did that as a dry run against the #3235 head a3802ba: it conflicts in 8 files, and what this PR keeps after the merge in the draft area is the fp4 clause in the resolver, one clause in the offload refusal, a few test rows and one doc sentence. The CPU tests of both areas on the merge result show only failures that also occur on this head without the merge (listed below). Until #3235 merges both PRs carry the mechanism; I did not stack the flag into this PR, which would add about 460 lines to its diff.
Asking: is merging main into this branch after #3235 lands acceptable, or do you want it stacked now?
KvGeometry.layout is typed KvLayout | None (pool_rows.py).
The shared MHA builder no longer imports Mxfp4KvLayout or tests the dtype in _module_kinds: SparseMHAPagedAttentionImpl.kv_layout declares the layout (None unless fp4) and the builder reads module.impl.kv_layout. The builder's _index_cache_spec still names the MXFP4 index page (PAGE_BYTES, Mxfp4KvLayout().check); I did not change that.
Not changed, asking for your explicit OK to defer each, with an issue I would open on your OK (nothing is created yet):
B5, segment loop (sparse_attn_mxfp4.py:108, about 28 lines) and plan_segment (base_attention.py:502, about 7 lines): replacing them by a dtype branch in run_pa_decode is about -35 lines here, but it needs MXFP4 K/V scale arguments in run_pa_decode (a change in base_attention.py and in aiter's pa_decode entry) and a gfx950 re-run of the sparse PA tests and GSM8K, since it is the decode path of every fp4 request. Proposed issue: "MiniMax-M3.1 MXFP4 KV: route sparse PA decode through run_pa_decode and drop the segment plan loop".
B7/B8, MhaKvPool takes kv_dtype and layout (mha_kv_pool.py:204-208) and transfer_pages reports self.kv_dtype (:379): dropping kv_dtype is about 35 source lines and 20 test call sites, and transfer_pages must then take the dim and dtype of each region from the layout; for an fp4 pool that changes what is reported (today head_dim and uint8), which a P/D pair has to agree on. Proposed issue: "MhaKvPool: take only the KV layout; the layout reports the transfer unit".
B13, per-model fp4 allow-list _check_fp4_kv (config.py:1948): "every other model refuses fp4" has to live in shared code; moving the M3 and V4.1 knowledge out needs a new per-model hook (about 40 lines). Proposed issue: "A per-model supports_fp4_kv hook instead of _check_fp4_kv in config.py".
The two skip-index writers (the Triton writer for non-fp4 skip layers, and aiter's skip_index_branch for fp4 KV in attention_mha.py): one writer needs a gfx950 comparison of the two on the same input and a decision on which page format a skip layer under fp8 KV + fp4 index uses. Proposed issue: "M3 skip-index layers: one fused writer for every KV dtype".
4. Docs docs/configuration_guide.md now lists, in the kv_cache_dtype row, decode context parallelism (--decode-context-parallel-size > 1) and a speculative draft that would keep FP4 KV (an MTP draft shares the target's pool; a DSpark or EAGLE3 draft with its own pool is bf16 and runs), and in the index_cache_dtype row the DCP refusal (015f202). The rules are the ones in models/minimax_m3/config.py:69 and _mxfp4_kv_refusal.
CPU checks at 015f202: black and ruff clean on the changed files. Two test files cannot be collected on that host (tests/test_dspark.py, test_dspark_swa_fp8_2buff.py: aiter stub). The same failures occur without this commit's changes: test_spec_lossless_staging.py, test_paged_attention_dispatch.py::TestDenseDecodeSplits, test_deepseek_v4_wo_a_dequant.py, test_deepseek_v41_mono_hi_gate.py, test_minimax_m3_flydsl_optional.py (on the CPU host the image's aiter has no aiter.ops.triton.fused_kv_cache in a bare interpreter; it imported fine on the GPU host). Size: 47 files, +2837/-412 before, +2848/-420 now (the new lines are the doc sentences and the impl's kv_layout; the builder lost its M3 branch).
…edits of the last commit
pool_layout/ may not import atom (tests/test_layout_packages.py), so the layout
annotation is a one-attribute Protocol defined beside KvGeometry instead of an
import of mha_kv_pool.KvLayout. The three comment corrections that belong to the
draft-KV PR are dropped here, and the docs wording and the impl's kv_layout
comment are shorter.
Co-Authored-By: Claude <noreply@anthropic.com>
My previous push (015f202) broke tests/test_layout_packages.py: pool_layout/pool_rows.py imported atom for the KvLayout annotation, which that package promises not to do. This is our regression, found after I posted; the "Run non-GPU unit tests" job failed because of it.
Fix (bd8fd40): KvGeometry.layout is annotated with a one-attribute Protocol (tag) defined next to KvGeometry, so there is no import. tests/test_layout_packages.py is unchanged and passes (31 passed; it fails on 015f202).
7fe048d is now included, so the unpushed note in my earlier comment no longer applies: the graph variant of the padded-step index test no longer allocates inside the capture (test_minimax_m3_mxfp4_index.py, 10 passed on gfx950).
Checks on a CPU host: black and ruff clean on the changed files; the touched-area tests give the same failures as before this round (environment ones, also present at 2808171).
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because an unpublished finding identifies a split-prefill configuration that passes validation but can fail at runtime with either MXFP4 cache mode.
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Action required before merge (5)
Drop the new getattr defense in the shared MHA builder.aiter_attention.py:424 reads getattr(getattr(module, "impl", None), "kv_layout", None). Every real module that reaches this branch sets self.impl (paged_attention.py:122), so the outer getattr exists only for the test stub _PagedAttention in tests/test_draft_kv_builder.py:29, which has no impl. The inner one hides impls that don't declare the attribute. Fix: declare kv_layout = None once on the impl base (PagedAttentionImpl, attention_mha.py:43), read module.impl.kv_layout, and give the stub an impl.
Merge this only after #3235.#3235 is still open with changes requested, and both PRs still carry draft_owns_kv_pool, DRAFT_POOL_FAMILIES, SpeculativeConfig.draft_kv_dtype and the proposer / calibration / hash edits. To answer your question: merging main in after #3235 lands, keeping only the fp4 -> bf16 clause, is fine. Please don't merge this one first.
Open the four follow-up issues and link them in the description. I'm OK deferring B5 (segment loop at sparse_attn_mxfp4.py:108 plus plan_segment at base_attention.py:502; it needs aiter's run_pa_decode to take MXFP4 scales), B7/B8 (MhaKvPool takes both kv_dtype and layout, mha_kv_pool.py:204-208), B13 (_check_fp4_kv, config.py:1948) and the two skip-index writers, as you proposed. Each one needs a linked issue first.
Explain the KV allocation mismatch you saw on every fp4 start.expected=169GB, actual=136GB (about 20%) is the model_runner.py:2517 sizing check firing. Start the same server with --kv-cache-dtype fp8. If the mismatch happens only with fp4, fix the fp4 pool or index-cache pricing here, because fp4 is losing KV blocks it already reserved. The GPU results you posted at 015f20238 are accepted for this head: the only changes since are an annotation, a test and docs.
Fix two test leftovers.tests/test_minimax_m3_mxfp4_cache.py:148 says "refusal elsewhere: test_v4_*", but no test refuses --kv-cache-dtype fp4 for a third model. Add pytest.raises for _check_fp4_kv("fp4", is_m3=False, is_v41=False). Also, tests/test_minimax_m3_emit_pad_row.py:4 still says the gfx950 variant is "GPU-PENDING, not yet run", but you ran it on MI355X.
Re-reviewed at bd8fd40. Already fixed: GPU run at 015f202 (gfx950 pytest 187 passed, the one failure was a test bug fixed in 7fe048d; GSM8K M3.1 TP4 fp4 + DSpark, 200 questions, 0.960 / 0.900; TBO smoke; EAGLE3 draft-dtype test); KvGeometry.layout typed through a Protocol with no atom import; the shared builder no longer imports Mxfp4KvLayout; the DCP and fp4-draft refusals are documented in configuration_guide.md:42-43. Not blocking: an omitted --index-cache-dtype follows the KV dtype (config.py:2969), and an omitted KV stays bf16. No aiter probing in product code. Black, Ruff and non-GPU unit tests pass; GPU accuracy jobs are pending.
…ce parallelism; test leftovers
PagedAttentionImpl.kv_layout = None, read as module.impl.kv_layout by the shared
builder (no getattr); the test stub gets an impl. The fused MXFP4 writer checks
the qkv row width including the index columns, which Ulysses drops for a top-k
reusing layer, so fp4 KV is refused with --sequence-parallel-size > 1 (docs and
test row). Add the refusal test for a third model; the emit_pad_row docstring no
longer says the gfx950 variant is unrun.
Co-Authored-By: Claude <noreply@anthropic.com>
1. The getattr in the shared builder: fixed in ececbeb. PagedAttentionImpl.kv_layout = None is declared once on the impl base, the builder reads module.impl.kv_layout, and the _PagedAttention stub in tests/test_draft_kv_builder.py has an impl.
2. Order with #3235: #2962 lands first and #3235 after it, as agreed with the maintainers; nothing to merge here for #3235.
3. Follow-up issues: opened, and linked in the PR description under "Follow-ups" (the rest of the description is unchanged).
4. KV cache allocation mismatch: not fixed, and I have not been able to run the fp8 comparison: all eight GPUs are in use by someone else's TP8 server, and I am not allowed to take them. I will run the same server with --kv-cache-dtype fp8 when four cards are free. What the fp4 logs already show (DSpark and no-DSpark runs, TP4):
The check compares the rank's own plan (pool_plan.total_reserved_bytes) with the bytes the rank allocated. The ranks sized different budgets: non_torch was 45.8 / 45.5 GB on two ranks and 17.2 / 16.9 GB on the other two, giving 98454 / 98647 / 118062 / 118255 blocks. Every rank then allocated 98454 blocks, the count the engine took from get_num_blocks (engine_core.py:146, then allocate_kv_cache(num_blocks)), which is the smallest of the four here.
The allocated bytes are exactly that count times block_bytes (143.656 GiB = 98454 x 1566720 B), and only the two ranks whose own plan was larger (172.3 / 172.5 GiB) print the warning. So in these runs fp4 is not losing blocks it reserved: the pool is priced the same on the plan and the allocation side. The open question is why non_torch differs by about 28 GB between ranks; I have not found it, and I do not know yet whether fp8 shows the same.
5. Test leftovers: fixed in ececbeb. test_what_still_passes_and_v41_keeps_its_fp4_kv now has pytest.raises for _check_fp4_kv("fp4", is_m3=False, is_v41=False); the docstring of test_minimax_m3_emit_pad_row.py no longer says GPU-PENDING (the gfx950 variant ran on MI355X and passed).
Copilot's two unpublished findings
(a) Index columns in the FP4 KV / Ulysses writer path (review 5874692436): real, in one configuration. The aiter fused writer checks qkv.size(1) == (nq + 2*nkv + niq + 1) * 128 and requires num_index_heads > 0 even with skip_index_branch=True (csrc/kernels/fused_qknorm_idxrqknorm.cu, expected_row), while SparseMHAPagedAttentionImpl.qkv_fields drops the index columns of a top-k reusing layer, so under --sequence-parallel-size > 1 the first reusing layer would fail that check. Fixed in ececbeb by refusing --kv-cache-dtype fp4 with sequence parallelism at startup (4 lines, a test row and a doc sentence). Not run on a GPU; the check itself is what I read in aiter, not something I executed.
(b) Split prefill passing validation and failing at runtime (review 5875178617): not confirmed. I looked at the prefill metadata of a split (TBO) prefill: make_sparse_prefill_metadata drops the work map and keeps only the score workspace, which both micro-batches use one after the other on the compute stream, and n_valid_column_per_row is allocated per call. I found no path that fails, but I did not run --enable-tbo (prefill only) with fp4 on a GPU, so I cannot call it not real; it is pending a GPU, and the finding has no details to check against.
CPU checks at ececbeb: black and ruff clean on the changed files; tests/test_layout_packages.py and the touched-area tests give the same failure set as before (environment ones, also present before this round). Size against main: 47 files, +2851/-413 (+2841/-413 at bd8fd40): +10 lines, of which the sequence-parallelism refusal with its test and doc sentence is 6 and the two test leftovers 4.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because both MXFP4 caches accept context-parallel prefill with TBO at startup but can hit an assertion when split-prefill query rows no longer match sparse metadata counts.
@valarLip answer to your item 4 (the KV cache allocation mismatch), measured. No code change, nothing pushed for it.
Cause: not the fp4 pool, and not a bug in this PR. My earlier GPU runs set HIP_VISIBLE_DEVICES assuming HIP index order equals PCI order. It does not on that machine (HIP index follows the KFD node order), so two of the four ranks ran on cards that another job on the same machine was also using, with about 28 GB of its own memory resident there. That memory shows up in non_torch ((total - free) - memory_reserved, model_runner.py:2051) on exactly those two ranks, 45.5 / 45.8 GB against 17.2 GB on the other two, and the difference is the 28 GB. The two ranks with the larger budget then print the warning, because every rank allocates the block count the engine took from rank 0 (async_proc.py:613, engine_core.py:146), not its own.
Re-run on four free cards (the same M3.1 TP4 + DSpark server as the GSM8K run, ececbeb, only the KV and index dtype changed; HIP indices derived from the KFD order, the other cards idle):
non_torch per rank (GB)
blocks per rank
block_bytes
mismatch warning
--kv_cache_dtype fp8 --index-cache-dtype fp8
17.16 / 17.54 / 17.45 / 17.45
57265 / 57138 / 57170 / 57170
3179520
none
--kv_cache_dtype fp4 --index-cache-dtype fp4
17.53 / 17.43 / 17.15 / 17.43
95835 / 95888 / 96049 / 95888
1894400
none
Post-init device memory is 251.9 to 252.6 GB on every rank in both runs. So fp4 prices the pool the same way on the plan and the allocation side, and loses no blocks it reserved; fp4 holds 1.67x the blocks of fp8 for the same memory.
Two corrections to my previous comment: the 98454 blocks that every rank allocated was rank 0's count, not "the smallest of the four" (it was the smallest only because rank 0 sat on a shared card), and the GSM8K / TBO numbers at 015f202 above came from that same shared-card setup. Accuracy does not depend on it; I did not measure throughput.
Not changed: the engine uses rank 0's count without taking a minimum across the TP ranks. With uniform ranks the counts differ by 0.2% and the 2% safety margin covers it; I left it alone.
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Action required before merge (3)
Close Copilot's split-prefill finding on a GPU (review at ececbeb). Copilot says both MXFP4 caches pass startup with context-parallel prefill + TBO but can hit the covered <= q.shape[0] assert in _metadata_rows (attention_mha.py:1676) when split-prefill rows no longer match the sparse metadata. You marked it "pending a GPU". Fix: run M3.1 TP4 fp4 with --enable-tbo (prefill split) and post the result, or refuse that combination in validate_runtime_config (models/minimax_m3/config.py:57) with a refusal row like the DCP/SP ones.
Get CI green at this head.Offline inference smoke test failed on a runner infra error (docker: unauthorized: authentication required), not on this PR. Rerun it. The heavy accuracy jobs, including MiniMax-M3-MXFP4, are still pending.
Rename the test that now also checks refusal.tests/test_minimax_m3_mxfp4_cache.py:147test_what_still_passes_and_v41_keeps_its_fp4_kv now also asserts that every other model is refused. Rename it, e.g. test_fp4_kv_is_m3_and_v41_only.
Merge order: as agreed, this PR lands before #3235; #3235 then rebases onto it and drops its copy of the shared draft-pool mechanism (draft_owns_kv_pool, DRAFT_POOL_FAMILIES, SpeculativeConfig.draft_kv_dtype).
Re-reviewed at ececbeb. Already fixed: the builder getattr (PagedAttentionImpl.kv_layout = None at attention_mha.py:48, read as module.impl.kv_layout at aiter_attention.py:424, test stub has an impl); follow-ups #3260/#3261/#3262/#3263 opened and linked; KV allocation mismatch explained (the GPUs were shared; on free cards, fp8 and fp4 both start with no warning, and fp4 gets 1.67x the blocks); the third-model refusal test and the stale GPU-PENDING docstring; the Ulysses SP refusal for fp4 KV (Copilot), with a test row and docs. No aiter probing, no shuffle interfaces, no test-only getattr in product code.
…ontext parallelism
Conflicts with the DSpark GQA draft (#2581): draft_kv.py keeps this branch's
family table and settled draft dtype and takes main's DraftKvPoolSpec and the
pool spec kwargs; MhaKvPool keeps the KvLayout and takes main's flash_layout
(no scale planes, fa_views); backends.py docstring; test stubs follow the table
(no DRAFT_OWNS_KV_POOL) and the settled draft dtype.
Refuse --prefill-context-parallel-size > 1 with either MXFP4 cache: a PCP
rank's q rows are a shard of what the sparse metadata covers, which
_metadata_rows asserts. Rename the third-model refusal test.
Co-Authored-By: Claude <noreply@anthropic.com>
@valarLip re-read of your review 5884830762. New head: bc61ed4 (a merge commit: main merged into this branch, no rebase, no force push; the branch was conflicting with main after #2581).
Merge conflicts (5 files, all from the Kimi-K3 DSpark GQA draft, #2581, which touches the same draft-pool code):
atom/spec_decode/draft_kv.py: kept this branch's family table (draft_owns_kv_pool, DRAFT_POOL_FAMILIES) and the draft dtype settled by Config, took main's DraftKvPoolSpec, draft_kv_dtype() and the pool-spec kwargs. A draft that declares a pool spec on a family without a draft pool is refused, as on main, now by the table instead of DRAFT_OWNS_KV_POOL. The scale binding follows the pool (pool.scale_fields).
atom/model_ops/attentions/mha_kv_pool.py: kept the KvLayout, took main's flash_layout (checked and priced as SHUFFLE unless FLASH: no scale planes, fa_views).
tests/test_draft_kv_builder.py, tests/test_dspark.py: both sides kept; the stubs follow the table and the settled dtype (tests/test_kimi_k3_dspark_gqa.py needed the same).
1. Split-prefill finding: refused, not run. I did not reproduce it: my earlier TBO runs used short prompts and I did not verify that a prefill split happened, and no GPU run was done for this round. The assert in _metadata_rows compares the sparse metadata's row count with q.shape[0], and under prefill context parallelism a rank's rows are a shard. So validate_runtime_config now refuses --prefill-context-parallel-size > 1 with either MXFP4 cache (test rows for both, doc line in configuration_guide.md), in bc61ed4.
3. Rename: test_what_still_passes_and_v41_keeps_its_fp4_kv is now test_fp4_kv_is_m3_and_v41_only (no other references).
2. CI: the offline smoke failure was the runner's docker: unauthorized, not this PR. I will rerun only that job once if it fails again after this push; the heavy accuracy jobs are still pending.
CPU checks: black clean; ruff clean on the files this PR changed; tests/test_layout_packages.py and the touched-area tests give the same failures as before the merge (environment ones), plus test_kimi_k3_dspark_gqa.py::test_the_builder_declares_draft_slots_only_under_dcp[8-True], which needs a GPU for a pinned buffer.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The new MXFP4 paths and shared draft/graph contracts lack current-head gfx950 validation, including TBO replay and mixed-phase DP execution, so fresh GPU results require human review.
No open issues 🎉
Review effort: Balanced
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MiniMax-M3.1 gets two MXFP4 caches on gfx950:
--index-cache-dtype fp4: the indexer's key cache is stored as MXFP4 pages (e2m1 data, one e8m0 scale per 32 elements, 8704 B a 128-token page), written by aiter's fused QK-norm / rope / cache writer, scored by aiter's FlyDSL scorers (an fp8 index query, e4m3), then selected by the same Triton selector as the other index caches or, on wide rows, aiter's per-row selector. This one can be used alone: the K/V cache staysbf16orfp8.--kv-cache-dtype fp4: K and V are MXFP4 pages of the same format, attended bysparse_pa_mxfp4(aiter's FlyDSLpa_decodewith MXFP4 K/V, an e4m3 query, one row per (token, KV head)). It needs the FP4 index cache too, because the attention reads exactly the rows the selection wrote and only the MXFP4 index scorer is sized to those rows.Scope: MiniMax-M3.1 (every attention layer an indexed sparse one). Plain MiniMax-M3 (a checkpoint with dense attention layers) is not supported for FP4 KV and is refused at startup (see the table); DeepSeek-V4.1's own FP4 KV is unchanged, and DeepSeek-V4 and every other model refuse
--kv-cache-dtype fp4. Not in this PR: NVFP4, context parallelism over the MXFP4 caches (refused), KV offload / P-D transfer of MXFP4 pages (refused), the vLLM/SGLang bridges (refused), speculative drafts with FP4 KV (a draft with a pool of its own is bf16; a draft in the target's pool is refused).--spec-decode-kv-cache-dtypeis NOT part of this PR: it was split out on review into #3235 (see "Related PRs"). What an FP4 target needs of that code stays here.What is accepted and what is refused at startup
Rules live in
atom/models/minimax_m3/config.py(validate_runtime_config) anddocs/configuration_guide.md, and nowhere else. Nothing falls back to FP8.--kv-cache-dtype--index-cache-dtypeEither cache set to fp4 also requires: native ATOM (refused under the vLLM/SGLang bridges),
--block-size 128, a gfx950 GPU (read from the live device, not fromGPU_ARCHS).--kv-cache-dtype fp4further requires:sparse_attention_configwithsparse_attention_freqexactly 1 for each ofnum_hidden_layers(so MiniMax-M3's original checkpoints with dense attention layers are refused), a bf16 model, no KV offload and no--kv-transfer-config, and no speculative draft left with FP4 KV. Context parallelism over either MXFP4 cache is refused byindexer_cp_unsupported_reason.Other models:
_check_fp4_kvinConfig.__post_init__allows--kv-cache-dtype fp4for the MiniMax-M3 architecture and DeepSeek-V4.1 only (message: "MiniMax-M3.1 and DeepSeek-V4.1 only"; the M3 hook then refuses what is not M3.1); every other model is refused.Files
atom/config.py,atom/model_engine/arg_utils.py,atom/models/minimax_m3/config.py(the new hook),docs/configuration_guide.md.atom/model_ops/minimax_m3/geometry.py,mxfp4_pages.py(page layout),index_buffers.py(the scorer's workspace and work map, one record per prefix),index_topk.py(scorers, selection, block-table emission),sparse_attn.py,sparse_attn_mxfp4.py.attention_mha.py(writer, KV/index paths, the row rule_metadata_rows),attentions/aiter_attention.py(index cache spec, scorer allocation, TBO),attentions/mha_kv_pool.pyandpool_layout/pool_rows.py(KvLayout: SHUFFLE or MXFP4 pages),base_attention.py(plan_segmentin the plan key),backends.py.spec_decode/draft_kv.py,utils/selector.py(DRAFT_POOL_FAMILIES), the DSpark / EAGLE proposers and the calibration identity readSpeculativeConfig.draft_kv_dtype; DeepSeek-V4's fp8 -> bf16 KV fallback is decided inConfig(it was in the V4 builder) so the draft's dtype is settled after it.tests/test_minimax_m3_mxfp4_{cache,index,attention,write,decode_batch}.py,test_minimax_m3_flydsl_optional.py,test_minimax_m3_emit_pad_row.py, (the index-cache-spec test file was dropped in the trim); changed:test_minimax_m3_index_topk.py,test_draft_kv_builder.py,test_dspark*.py,test_indexer_cp_gate.py,test_kv_pool_contract.py,test_v4_indexer_defaults.py,test_arg_utils_spec.py, lmcache/pp-pd stubs (their fake builders follow the new index-cache spec).Net diff against main: see the PR header (head
880044c46); the tests this PR adds were halved in the last push (one test per behaviour).Other behaviour changes in this PR (outside the MXFP4 paths)
_emit_sparse_block_table_rowclamps a row's causal length where it enters, so a pad row (length 0, q > 1) computes nothing from a negative length. The helper is shared by every index cache (bf16/fp8 Triton, MXFP4, candidate exchange); live rows are unchanged.aiter.ops.flydsl.topk.topk_per_row_small_k(main's flat import path does not exist in a current aiter, so main ran Triton there), a bf16 / fp8 row of 131072+ tokens is served by aiter's selector, not Triton. Not measured for bf16 / fp8. An aiter without that module fails at that row (no fallback; see Dependencies).Config, not in the metadata builder.Dependencies
Minimum aiter (documented in
docs/configuration_guide.md, no code probe): a build with ROCm/aiter #7014 (topk_per_row_small_k, used by every M3 index cache on rows of 131072+ tokens), #7057 (fused writer into MXFP4 pages), #7060 and #7061 (MXFP4 index decode / prefill scorers), #7056 (MXFP4 K/V inpa_decode). Upgrade ATOM and aiter together. No new aiter API beyond these is required.Related PRs
--spec-decode-kv-cache-dtype(bf16 / fp8 KV for a draft with its own pool), requested vs settled draft dtype as two fields. Independent of this PR (based on main); whichever lands second is rebased on the other.Validation
What was run on a GPU, and at which head: the last server-level run was at
55070fb73; nothing server-level was run on this head, and 70+ commits and a merge of main came after it.55070fb73(the flag then spelled--kv-cache-dtype mxfp4 --index-cache-dtype mxfp4; the fp4 spelling came later): TP4, DSpark with 7 draft tokens, MXFP4 KV and index: 279 GPU tests passed, 32 skipped; GSM8K (all 1319 questions) flexible 0.9431 +/- 0.0064, strict 0.8613 +/- 0.0095, 12 empty responses; TP2--enable-tbo allsmoke on 300 questions, flexible 0.9633, strict 0.9200. Earlier heads (13f413d0b,bb1719ea9): GSM8K 0.9416 / 0.8825 and 0.9469 / 0.8704 (TP4), padded-replay and TBO smokes (TP2), AgentX c32 368.92 tok/s/GPU on13f413d0b; these are older code and are not evidence for this head.55070fb73: kernel-level checks on one MI355X only (bit-equal selections with and without the unfilled workspaceout=, arun_perftesttable of scorer + selector, contiguous vs aiter's layout), at aiter5469555096, posted in the PR comments; not re-run at this head.test_config*,test_spec*,test_mtp_*,test_lmcache*: 20 failed / 1798 passed / 209 skipped / 2 collection errors (head880044c46). The 20 failures and 2 collection errors are the same ids as on the unmodified head, except that the FlyDSL-optional test was merged into one subprocess test, which fails in that image for the same missing aiter module as the old one it replaces (missing aiter modules in that image:test_spec_lossless_staging,test_minimax_m3_flydsl_optional, ...). The gfx950-gated tests (the real-kernel writer, attention, index and decode-batch tests, and the compiled variant oftest_minimax_m3_emit_pad_row.py) are skipped there and have not run at this head.--kv_cache_dtype fp8, so a green CI says nothing about the fp4 paths.Not validated on any GPU at this head: the FP4 KV and index end to end; TBO decode output parity and split-bucket graph replay; a DP group with one rank prefilling while another decodes; an MHA EAGLE3 / DSpark draft under an fp4 target; V4's fp8 -> bf16 fallback on gfx942; the long-context (131072+) bf16 / fp8 rows on aiter's selector; the aiter fused-writer change (one name table, B15).
Known limits
max_model_len, not the batch's context (B1, B3). The workspace isH x max(max_bs*max_qlen, max_num_batched_tokens) x cdiv(max_model_len, 128) x 4 B(512 MiB at TP4 and 1M context, 4x at TP1), allocated at builder construction, so the memory profile counts it._FLYDSL_PLAN_MAX_BATCH) and has no gluon fallback; eager decode plans accumulate per distinct row count in the process-wide plan cache (B4, B5).MhaKvPoolstill takes bothkv_dtypeand aKvLayout(B7, B8); the shared builder and the attention impl both decide "M3 sparse => MXFP4"; the per-model fp4 allow-list sits inconfig.py(B13); two skip-index writers exist (Triton for a non-fp4 KV skip layer, aiter'sskip_index_branchfor fp4 KV).Follow-ups
Deferred with the maintainer's agreement, one issue each:
run_pa_decodeand drop the segment plan loop (sparse_attn_mxfp4.py,plan_segment).MhaKvPooltakes only the KV layout; the layout reports the transfer unit._check_fp4_kvinconfig.py.skip_index_branch) become one.🤖 Generated with Claude Code